Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WEB: Implement managed service cloning & notification ui & config/secrets ui #226

Merged
merged 4 commits into from
Jun 14, 2024

Conversation

nxtcoder19
Copy link
Contributor

@nxtcoder19 nxtcoder19 commented Jun 14, 2024

Resolves kloudlite/kloudlite#250

Summary by Sourcery

This pull request introduces several new features including managed service cloning, a notification UI, and enhanced configuration and secrets management UI. It also includes significant refactoring to improve code modularity and updates to the GraphQL schema to support these new features.

  • New Features:
    • Implemented UI for managed service cloning, allowing users to clone existing managed services.
    • Added notification UI to display user notifications and mark them as read.
    • Introduced configuration and secrets management UI with enhanced editing capabilities.
  • Enhancements:
    • Refactored resource handling components to use a more modular and reusable structure.
    • Updated the GraphQL schema and queries to support new features like managed service cloning and notification management.
    • Improved the layout and interaction of the configuration and secrets management interfaces.

- fix managed service loading previous account data while switching
  account
- fix byok cluster setup instruction fetching error while switching
  account
Copy link

sourcery-ai bot commented Jun 14, 2024

Reviewer's Guide by Sourcery

This pull request implements managed service cloning, notification UI, and configuration/secrets UI. The changes include refactoring existing components, adding new components for handling notifications, and updating GraphQL queries and mutations to support the new features.

File-Level Changes

Files Changes
src/apps/console/routes/_main+/$account+/env+/$environment+/secret.$secret/resources.tsx
src/apps/console/routes/_main+/$account+/env+/$environment+/config.$config/resources.tsx
src/apps/console/routes/_main+/$account+/env+/$environment+/secret.$secret/handle.tsx
src/apps/console/routes/_main+/$account+/env+/$environment+/config.$config/handle.tsx
Refactored resource components to use new icons and components, added new types and components for handling resource actions, and updated form validation to support updates.
src/apps/console/routes/_main+/$account+/managed-services/backend-services-resources-V2.tsx
src/apps/console/routes/_main+/$account+/managed-services/clone-managed-service.tsx
Added new components and logic for cloning managed services.
src/apps/console/routes/_main+/$account+/env+/$environment+/new-app/app-detail.tsx
src/apps/console/page-components/app/general.tsx
Updated form validation for image URL and refactored form fields to support new validation.
src/apps/console/routes/_main+/$account+/infra+/byok-cluster/handle-byok-cluster.tsx
src/apps/console/routes/_main+/$account+/infra+/clusters/cluster-resources-v2.tsx
Added new logic to handle cluster visibility settings and ensure account is set client-side.
src/apps/console/routes/_main+/$account+/env+/$environment+/config.$config/route.tsx
src/apps/console/routes/_main+/$account+/env+/$environment+/secret.$secret/route.tsx
Added new logic to handle config and secret updates, and updated form validation to support updates.
src/apps/console/routes/_main+/$account+/user-profile/_layout.tsx
src/apps/console/routes/_main+/_layout/_layout.tsx
Enabled notifications tab in user profile and implemented NotificationMenu component to display notifications.
src/apps/console/server/gql/queries/cluster-managed-services-queries.ts
src/apps/console/server/gql/queries/comms-queries.tsx
src/apps/console/server/gql/queries/byok-cluster-queries.ts
src/apps/console/server/gql/queries/cluster-queries.ts
src/apps/console/server/gql/saved-queries.ts
gql-queries-generator/doc/queries.graphql
src/generated/gql/server.ts
src/generated/gql/sdl.graphql
Added new GraphQL queries and mutations for handling notifications and managed service cloning, and removed deprecated fields from existing queries and types.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

@nxtcoder19 nxtcoder19 requested review from tulsiojha and removed request for karthik1729 and abdheshnayak June 14, 2024 11:26
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nxtcoder19 - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 10 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 2 issues found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@@ -1,51 +1,30 @@
import {
DotsThreeVerticalFill,
Eye,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Unused import 'generateKey' removed.

Comment on lines -44 to -45
interface IResourceItemExtraOptions {
onDelete: (() => void) | null;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Removed unused interface IResourceItemExtraOptions.

Comment on lines -61 to -63
const ResourceItemExtraOptions = ({
onDelete,
onRestore,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Removed unused component ResourceItemExtraOptions.

Suggested change
const ResourceItemExtraOptions = ({
onDelete,
onRestore,
const ResourceItemExtraOptions = () => {
return null;
}

Comment on lines -131 to -138
const RenderItem = ({
item,
onDelete,
onEdit,
onRestore,
onShow,
edit,
listMode = true,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Removed unused component RenderItem.

import { useEffect, useState } from 'react';
import AnimateHide from '~/components/atoms/animate-hide';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Unused import 'generateKey' removed.

Comment on lines -120 to -126
const RenderItem = ({
item,
onDelete,
onEdit,
onRestore,
edit,
listMode = true,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Removed unused component RenderItem.

Comment on lines +103 to +105
imageUrl: Yup.string().matches(
/^\w(\w|[-/])+?(?::(\w|[-])+)?\w$/,
'Invalid image format'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Added validation for imageUrl format.

Suggested change
imageUrl: Yup.string().matches(
/^\w(\w|[-/])+?(?::(\w|[-])+)?\w$/,
'Invalid image format'
imageUrl: Yup.string().url('Invalid image format'),

@@ -171,6 +174,8 @@ const ByokInstructionsPopup = ({
const ByokButton = ({ item }: { item: CombinedBaseType }) => {
const [show, setShow] = useState(false);

console.log('item', item);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Remove console.log statement.

Consider removing the console.log statement before merging.

Suggested change
console.log('item', item);
return (
<div>
{show ? (

@@ -5,9 +5,9 @@ import { parseName } from '~/console/server/r-utils/common';
import { FadeIn } from '~/console/page-components/util';
import { NameIdView } from '~/console/components/name-id-view';
import { BottomNavigation, GitDetailRaw } from '~/console/components/commons';
import { registryHost } from '~/lib/configs/base-url.cjs';
// import { registryHost } from '~/lib/configs/base-url.cjs';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider removing commented-out code and simplifying validation logic.

The new code introduces several complexities that could be avoided:

  1. Commented Out Code: The presence of commented-out code (e.g., RepoSelector) clutters the codebase and can be confusing for future developers. It's better to remove unused code entirely.

  2. Inline Validation: The inline validation for imageUrl using Yup.string().matches() is harder to read, especially with complex regex patterns. Consider simplifying or documenting the regex for clarity.

  3. Redundant Code: The TextInput component for imageUrl is added directly in the JSX, but the commented-out RepoSelector code remains. This makes it unclear whether RepoSelector is intended to be removed or reused.

  4. Error Handling: Error handling for imageUrl is done directly in the JSX, which can be less clear than a more centralized approach.

Consider refactoring to remove unnecessary comments, simplify validation logic, and handle errors more clearly. This will make the code easier to read, understand, and maintain.

@@ -5,12 +5,12 @@
import { FadeIn } from '~/console/page-components/util';
import { NameIdView } from '~/console/components/name-id-view';
import { BottomNavigation, GitDetailRaw } from '~/console/components/commons';
import { registryHost } from '~/lib/configs/base-url.cjs';
// import { registryHost } from '~/lib/configs/base-url.cjs';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider removing commented-out code to improve readability and maintainability.

The new code introduces several complexities that could be avoided:

  1. Commented Out Code: The code has several lines commented out instead of being removed, which clutters the codebase and makes it harder to read and maintain.
  2. Inline Regex Duplication: The regex for validating imageUrl is duplicated, making future updates error-prone. Consider defining the regex once and reusing it.
  3. Unnecessary Component Change: Replacing RepoSelector with TextInput might lead to a loss of functionality or user experience that RepoSelector provided.
  4. Error Handling: The new error handling within TextInput might not be as robust as the previous implementation.

Here is a suggested refactor to address these issues:

import { useAppState } from '~/console/page-components/app-states';
import useForm, { dummyEvent } from '~/root/lib/client/hooks/use-form';
import Yup from '~/root/lib/server/helpers/yup';
import { parseName } from '~/console/server/r-utils/common';
import { FadeIn } from '~/console/page-components/util';
import { NameIdView } from '~/console/components/name-id-view';
import { BottomNavigation, GitDetailRaw } from '~/console/components/commons';
import { useOutletContext } from '@remix-run/react';
import AppBuildIntegration from '~/console/page-components/app/app-build-integration';
import { keyconstants } from '~/console/server/r-utils/key-constants';
import { constants } from '~/console/server/utils/constants';
import { Button } from '~/components/atoms/button';
import { useEffect, useState } from 'react';
import { toast } from '~/components/molecule/toast';
import ResourceExtraAction from '~/console/components/resource-extra-action';
import { ArrowClockwise, GitMerge, PencilSimple } from '~/console/components/icons';
import { TextInput } from '~/components/atoms/input';
import { IEnvironmentContext } from '../_layout';
import { getImageTag } from './app-utils';
import BuildSelectionDialog from './app-build-selection-dialog';

const imageUrlRegex = /^\w(\w|[-/])+?(?::(\w|[-])+)?\w$/;

const ExtraButton = ({ onNew, onExisting }: { onNew: () => void; onExisting: () => void; }) => {
  // ... rest of the component code
};

const validationSchema = Yup.object({
  name: Yup.string().required(),
  displayName: Yup.string().required(),
  imageUrl: Yup.string().matches(imageUrlRegex, 'Invalid image format'),
  manualRepo: Yup.string().when(['imageUrl', 'imageMode'], ([imageUrl, imageMode], schema) => {
    if (imageMode === 'git') {
      return schema;
    }
    if (!imageUrl) {
      return schema.required().matches(imageUrlRegex, 'Invalid image format');
    }
    return schema;
  }),
  imageMode: Yup.string().required(),
});

// ... rest of the component code

<TextInput
  size="lg"
  label="Image name"
  placeholder="Enter Image name"
  value={values.imageUrl}
  onChange={handleChange('imageUrl')}
  error={!!errors.imageUrl}
  message={errors.imageUrl}
/>

// ... rest of the component code

This refactor removes commented-out code, avoids regex duplication, and maintains the existing functionality.

- implement clone managed service
- make ui changes in configs and secrets
@nxtcoder19 nxtcoder19 force-pushed the update/notification-ui branch from d3f7b76 to deea57c Compare June 14, 2024 11:33
@tulsiojha tulsiojha merged commit f8e260e into release-v1.0.5 Jun 14, 2024
4 checks passed
@nxtcoder19 nxtcoder19 deleted the update/notification-ui branch June 14, 2024 12:18
abdheshnayak pushed a commit that referenced this pull request Oct 28, 2024
WEB: Implement managed service cloning & notification ui & config/secrets ui
tulsiojha added a commit that referenced this pull request Nov 1, 2024
WEB: Implement managed service cloning & notification ui & config/secrets ui
abdheshnayak pushed a commit that referenced this pull request Nov 5, 2024
WEB: Implement managed service cloning & notification ui & config/secrets ui
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WEB: Implement cloning of managed service
2 participants